Skip to content

Various fixes for CodeChecker warnings#8938

Open
craftmaster1231 wants to merge 99 commits intoFirebirdSQL:masterfrom
craftmaster1231:master_fix_warns
Open

Various fixes for CodeChecker warnings#8938
craftmaster1231 wants to merge 99 commits intoFirebirdSQL:masterfrom
craftmaster1231:master_fix_warns

Conversation

@craftmaster1231
Copy link

This PR contains warning fixes contributed by several students.
Each student was assigned a specific set of warnings to resolve.
The warnings were generated using CodeChecker tool on a Release build.
Most fixes are self-explanatory.

See:
https://codechecker.readthedocs.io/en/latest/

Zakk0n and others added 30 commits August 27, 2025 20:50
The 'can_use_more' variable is now initialized with a default value at declaration. This resolves the "variable may be uninitialized" compiler warning that occurred when the initial 'length' was zero.
The 'long_value' variable was declared at the start of a loop without a default value. This created a risk of being used with a garbage value, triggering compiler warnings.
This prevents potential use of an uninitialized variable, silences static analysis warnings, and makes the code safer and more predictable, even if the problematic code path was not reachable under the current logic.
The 'length' variable was declared without a default value in a function with complex control flow (two separate switch statements).
Initialized variable 'n' in blr_print_verb to 0 to avoid undefined behavior and remove compiler warning.
The 'savNumber' variable was not initialized when declared. This resulted in a compiler warning and risk of using a garbage value in 'case Request::req_unwind' if the transaction was a system one (TRA_system).

Added default initialization ({}) to ensure predictable behavior and fix a bug.
The 'parentStream' variable was not initialized when declared. This could lead to a random value being used on the second and subsequent loop iterations in the pass1Erase function.

Added default initialization ({}) to ensure safe behavior.
The 'returnsPos' variable was being set in two separate but logically mutually exclusive 'if' blocks. This was causing a false compiler warning about the possible use of an uninitialized variable.

Added initialization of 'returnsPos = 0' on declaration to eliminate the warning.
The 'parentStream' and 'parentNewStream' variables were not initialized when declared. This caused the compiler to warn about possible use of their values before assignment on the second and subsequent iterations of the loop.

Added default initialization ({}) to both variables to ensure safe behavior.
The 'parentStream' variable was not initialized when declared. This was causing a compiler warning.

Added default initialization ({}) to the variable to ensure safe behavior.
The 'value' pointer was not initialized, which created a risk of dereferencing it with a random value in one of the branches of the conditional logic. This resulted in a compiler warning and potential undefined behavior.

Added initialization of the pointer to 'nullptr' to ensure a safe state.
In UnicodeUtil::Utf16Collation::stringToKey function, 'lastCharKeyLen' variable was left uninitialized if 'srcLenLong' was zero. This resulted in reading random value from memory and undefined behavior.

Added zero initialization for 'prefixLen' and 'lastCharKeyLen' when they are declared to fix the bug and eliminate compiler warning.
The 'merge_pool' pointer was initialized only under the condition 'count > 1'. Although its reading was protected by the same condition, the static analyzer issued a warning about the possible use of an uninitialized variable.

Added initialization of the pointer to the value 'nullptr' when declaring.
In CVT_get_double, the 'value' variable was left uninitialized if the 'default' block was executed in the switch statement. This caused the compiler to warn about the possible use of the variable before it was initialized.

Added 'value = 0.0' initialization on declaration to ensure safe behavior in all code paths.
The 'ret_code' variable was not initialized when declared in the do-while loop.

Added initialization 'ret_code = 0' to make the code more robust.
The 'minlen' variable was left uninitialized if the data type in 'to_desc' was not handled by any of the 'case' in the switch block. This resulted in reading a random value and undefined behavior.

Added initialization 'minlen = 0' on declaration to fix the bug.
lDeathlWhisperl and others added 27 commits October 7, 2025 13:56
1) Use bit shifts instead of division by 4
2) Replace % 4 with bitwise AND
3) Use bitwise &/| instead of logical &&/|| for branchless code
…HORT, and the constant initialization with the same value has been removed.
Fixed forming reference to null pointer
…ialized

Fix variable may be uninitialized
Added default initialization to avoid garbage values
void StatusVector::ImplStatusVector::assign(const Exception& ex) noexcept
{
clear();
ImplStatusVector::clear();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using of class name inside of the class' methods is completely pointless.

*
**************************************/
enum trig_t type;
enum trig_t type = trig_none;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value is better to be named trig_invalid and a check for valid type was assigned should be added.

const int y_1 = y - 1;
const int yy = y_1 % 100;
const int c = y_1 - yy;
const int g = yy + (yy >> 2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modern compilers are clever enough to reuse pieces of calculation and replace division by 4 with shifts, but division is better readable by human.

{
jrd_tra* transaction = request->req_transaction;
SavNumber savNumber;
SavNumber savNumber = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is better to move this declaration to the points of real use to make sure that savepoint zero is never used by any mistake.

args.pat_ident1 = blob->blb_len_ident;
args.pat_ident2 = blob->blb_buff_ident;
args.pat_string1 = global_status_name;
args.pat_long1 = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and in other places where variable type is SLONG: assign zero instead of NULL, please.

// The bug appears in TCS DSQL_DOMAIN_12 and 13
//
const double value = *var->value.asFloat;
const double value = static_cast<double>(*var->value.asFloat);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assignment of float to double doesn't lose precision. What is really fixed here?


const ConfigFile::Parameter* module = ch->sub->findParameter("intl_module");
const ConfigFile::Parameter* objModule;
const ConfigFile::Parameter* objModule = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nullptr instead of NULL, please.

else if (org_rpb->rpb_record) // we apply update to delete stub
else // we apply update to delete stub
{
fb_assert(org_rpb->rpb_record);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this change fb_assert below lost purpose and can be removed. The comment may be moved inside and preserved, but, please, erase reference to me from it.

struct temporary_mini_key
{
USHORT key_length;
USHORT key_length = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converting POD type to constructible one should be very well considered. Especially in this ancient and dark piece of code.

#endif

memset(sh_mem_mutex->mtx_mutex, 0, sizeof(*(sh_mem_mutex->mtx_mutex)));
memset(sh_mem_mutex->mtx_mutex, 0, sizeof(sh_mem_mutex->mtx_mutex));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is definitely wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants